-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tools: add workflow to update release links #50710
Conversation
Review requested:
|
/cc @nodejs/tsc. I think this requires adding write access to https://github.com/nodejs/release-cloudflare-worker for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh this is awesome! That's an interesting way of triggering the Workflow by using GitHub's CLI!
steps: | ||
- name: Trigger update-links workflow on nodejs/release-cloudflare-worker | ||
run: | | ||
gh workflow run update-links.yml --repo nodejs/release-cloudflare-worker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also trigger the job (after this one) to deploy the worker to prod 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you want this to happen, it should be done from the nodejs/release-cloudflare-worker repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, was pondering on that. @flakey5 it might make sense to make it autodeploz after updating the links
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it currently opens a PR and needs manual approval & merge - we will need to change it to commit and push to main - I don't know if that is possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it currently opens a PR and needs manual approval & merge - we will need to change it to commit and push to main - I don't know if that is possible
It is. You can make the workflow commit and push to main directly,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd definitely be for either letting the releasers merge the pr it generates in release-cloudflare-worker or having them automatically merged and then deployed. I'm not sure exactly how we could get the second option working right now though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is easy to do the 2nd. Just instead of opening a PR simply do a commit and then push. You can see a workflow that does similar on the nodejs.org repository (I think the i18n workflow)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that sounds good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we create an issue on the worker repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Landed in 86593e5 |
Refs: #50623 PR-URL: #50710 Reviewed-By: Claudio Wunder <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Refs: nodejs#50623 PR-URL: nodejs#50710 Reviewed-By: Claudio Wunder <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Refs: #50623 PR-URL: #50710 Reviewed-By: Claudio Wunder <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Refs: #50623 PR-URL: #50710 Reviewed-By: Claudio Wunder <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Refs: #50623